Fix: escape shortcut key overriding each other#49
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| // https://github.com/Expensify/App/issues/18480 | ||
| const strictIndex = _.findIndex(commands, item => ( | ||
| (item.input === json.input && matchesModifierFlags(item)) | ||
| ((item.input === json.input || (matchesEscape(item))) && matchesModifierFlags(item)) |
There was a problem hiding this comment.
This technically happens to every non single letter input, but I only added it for escape for now. Let me know if I should handle them all.
There was a problem hiding this comment.
I'm not sure I understand the logic here. Why are item.input and json.input not equal? (and what are their values)
There was a problem hiding this comment.
I'm still trying to understand the bug. Here is what I think is going: After you press ESC, we look for an exact (strict) match and we fail and fallback for loose match and we end up with the listener for SHIFT + ESC (because it's somehow first in the list). Does that sound about right? If so I think we should add a condition to handle exact ESC key combination i.e.:
const strictIndex = _.findIndex(commands, item => (
(item.input === json.input && matchesModifierFlags(item))
|| (matchesEnter(item) && matchesModifierFlags(item))
|| (matchesEscape(item) && matchesModifierFlags(item))
));There was a problem hiding this comment.
Ahh it's exactly the same logic your are making XD
Can you just rewrite that way as I think it's a little more readable
There was a problem hiding this comment.
Haha, I'm trying to keep it short. Updated.
|
cc: @s77rt for review |
|
cc @neil-marcellini please merge this |
3 participants
Same root cause as Expensify/App#18480 and same solution too.
Related issue: Expensify/App#61622
Added Shift + ESC shortcut to the example app. Both ESC and Shift + ESC works fine.
w.mp4